Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(crud): menu not fully showing for field actions, remove old backgrounds COMPASS-6186 #3574

Merged
merged 5 commits into from
Oct 13, 2022

Conversation

Anemy
Copy link
Member

@Anemy Anemy commented Oct 11, 2022

COMPASS-6186

Will remove the release notes label before merging as this isn't a released bug.
This PR fixes the menus not showing in the crud view. We're fixing it by removing the overflow: none in the KeylineCard component. I think this is better than changing the menu usePortal={true} for now. Currently when an element that a portal is referring to is removed from the dom, the menu teleports to the top left while it collapses. We could work around this, but I think usePortal={false} is a cleaner approach.
Did some drive by cleanup, removing old backgrounds.
Also added another 16px of margin to the bottom of the document list for a little better ux with the menus.

Because of these changes the previous box shadow on document cards being edited is now shown (it was hidden by the hidden overflow before). Do we want to remove this shadow? cc @mcasimir

Before After
Screen Shot 2022-10-11 at 2 34 39 PM Screen Shot 2022-10-11 at 2 35 16 PM

@mcasimir
Copy link
Collaborator

Ditching the overflow hidden and the background makes sense to me, good call! Let's also remove the shadow, if we want them back we can re-add them later together with design.

@Anemy Anemy requested a review from mcasimir October 12, 2022 15:31
@@ -11,12 +11,11 @@
margin-bottom: 5px;

.json-document-is-editing {
box-shadow: 2px 5px 8px fade(@palette__black, 20%);
border-radius: inherit;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice trick, didn't know that we could have done this!

@Anemy Anemy merged commit 27e900f into main Oct 13, 2022
@Anemy Anemy deleted the COMPASS-6186-fix-menus-in-crud branch October 13, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants